Upgrade to Java 17 and Spring Boot 2.7.18 (Maven + Gradle) - #58
tobydrinkall wants to merge 4 commits into
Conversation
Co-Authored-By: Toby Drinkall <toby.drinkall@cognition.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
📝 Info: Legacy H2 DDL kept intentionally, but H2 2.x compatibility is version-fragile
DROP TABLE customers IF EXISTS (trailing form) and the SERIAL type are legacy H2 grammar; they still parse in H2 2.1.x (managed by Boot 2.7.18) as the PR describes, but they are not standard and would break on a future H2 major bump. Modernising to DROP TABLE IF EXISTS customers and id IDENTITY/BIGINT AUTO_INCREMENT would remove that coupling to the managed H2 version.
(Refers to lines 72-73)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Left as-is deliberately. The point of the H2 note in the description is that the legacy grammar still parses on the version Boot 2.7.18 manages (2.1.214) — verified at runtime here. Modernising the DDL is a good follow-up but is a source change unrelated to the Java/Boot upgrade, so I'd rather it not ride along in this PR.
| RestTemplate restTemplate = new RestTemplate(); | ||
| Quote quote = restTemplate.getForObject("http://gturnquist-quoters.cfapps.io/api/random", Quote.class); | ||
| log.info(quote.toString()); | ||
| logRandomQuote(new RestTemplate()); |
There was a problem hiding this comment.
📝 Info: Quote is still fetched twice per startup
The quote is fetched once from the CommandLineRunner bean (with the builder-configured RestTemplate) and again from main() with a freshly constructed new RestTemplate(). This duplication is pre-existing, but now that both paths funnel through logRandomQuote, the dead host will produce two WARN lines on every start. Consolidating on the injected RestTemplate (and dropping the call in main) would be cleaner. Note also that Application itself implements CommandLineRunner (src/main/java/hello/Application.java:23) while also exposing a second CommandLineRunner bean named run, which is confusing but functional.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Pre-existing duplication, left as-is: both call sites are demo code from the original Spring guide, and consolidating them changes runtime behaviour (one fewer startup fetch) beyond the scope of a version upgrade. Worth noting the second WARN is now visible only because the host is dead; when the endpoint worked, both paths logged a quote. Happy to collapse main()'s call and the redundant run bean into the existing CommandLineRunner implementation if the repo owner wants it.
| implementation("org.springframework.boot:spring-boot-starter-web") | ||
| implementation("org.springframework.boot:spring-boot-starter-jdbc") | ||
| runtimeOnly("com.h2database:h2") | ||
| testImplementation("junit:junit") |
There was a problem hiding this comment.
📝 Info: properties-migrator kept on the runtime classpath in both builds
spring-boot-properties-migrator is intended as a temporary, single-upgrade aid and Spring's docs recommend removing it once the migration is done; it adds startup cost and log noise on every run. Now that the upgrade reports no renamed/removed properties, both the Maven runtime dependency and this new Gradle runtimeOnly entry could be dropped.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Aligned in 20c978f: added runtimeOnly("org.springframework.boot:spring-boot-properties-migrator") to build.gradle, so both jars now report renamed/removed properties. Keeping the migrator for this PR is deliberate — it's the only signal available for Boot 2.0 -> 2.7 property renames in a repo with no tests; dropping it is a sensible follow-up once the upgrade has been exercised. The h2 scope difference is intentional and not a behavioural divergence: Maven's h2 is compile-scope only because that's what the original pom had, and nothing in the source references H2 classes.
There was a problem hiding this comment.
Scopes now match exactly: h2 is <scope>runtime</scope> in the pom as of 2f4e18f, mirroring Gradle's runtimeOnly. Rebuilt and re-booted the Maven jar — GET / and the H2 table creation/insert/query still work, confirming nothing referenced H2 at compile time.
On the migrator: agreed it's temporary, and I'd remove it in a follow-up rather than here. This repo has no tests and no CI, so a startup report of renamed/removed properties is the only regression signal available while the Boot 2.0 -> 2.7 jump is being reviewed; dropping it in the same PR would remove that signal before anyone has exercised the upgrade.
There was a problem hiding this comment.
Position unchanged from my earlier reply in this thread: the migrator stays for this PR and comes out in a follow-up. "Reports no renamed properties" is only true for the paths I exercised by hand — with no tests and no CI, a reviewer running this branch is the next chance to catch a rename, and removing the migrator now removes that chance.
| <maven-compiler-plugin.version>3.13.0</maven-compiler-plugin.version> | ||
| <maven-surefire-plugin.version>3.5.6</maven-surefire-plugin.version> |
There was a problem hiding this comment.
📝 Info: Overridden plugin versions rely on parent property names
maven-compiler-plugin.version and maven-surefire-plugin.version only take effect because spring-boot-dependencies parameterises its pluginManagement with exactly those property names; if a future parent upgrade renames them the overrides silently become no-ops (the build would just use the parent's pinned versions). Also, with maven.compiler.release=17 set alongside the parent-derived source/target from java.version, release wins and source/target are ignored — harmless but redundant.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Confirmed effective, not inert. mvn help:effective-pom on this branch resolves maven-compiler-plugin to 3.13.0 and maven-surefire-plugin to 3.5.6 in both pluginManagement and build/plugins (parent 2.7.18 pins 3.10.1 / 2.22.2), so the property names match what spring-boot-dependencies parameterises.
There was a problem hiding this comment.
Keeping both. java.version is the property spring-boot-starter-parent documents, so it's what a reader (and any Boot tooling) expects to find; maven.compiler.release is the one that actually gives --release 17, which the parent has no property for. The redundancy is intentional rather than accidental. Agreed a future parent could rename the plugin-version properties — that's a general property-inheritance risk with spring-boot-starter-parent and would surface as the parent's pinned versions being used, not as a build failure.
Co-Authored-By: Toby Drinkall <toby.drinkall@cognition.ai>
Co-Authored-By: Toby Drinkall <toby.drinkall@cognition.ai>
| java { | ||
| toolchain { | ||
| languageVersion = JavaLanguageVersion.of(17) | ||
| } | ||
| } | ||
|
|
||
| tasks.withType(JavaCompile) { | ||
| options.release = 17 |
There was a problem hiding this comment.
📝 Info: Gradle toolchain requires a locally installed JDK 17
The java.toolchain block plus options.release = 17 means Gradle will fail with a toolchain-resolution error on machines without a JDK 17 installed, since no toolchain resolver plugin is configured. This is documented in the README, but it makes the Gradle build stricter than the Maven build, which compiles with whatever JDK runs Maven as long as it supports --release 17.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Documented in 8255bf2: the README's Prerequisites now say JDK 17 (it said "Java sdk"), Getting Started calls out that the Gradle toolchain will not download a JDK because no resolver is configured, and both the Maven and Gradle build/run command pairs are listed. Kept the toolchain block rather than adding foojay-resolver — failing loudly with "No matching toolchains found" is better than silently compiling against whatever JDK happens to be on PATH, and options.release = 17 alone would not pin the JVM that runs the build.
There was a problem hiding this comment.
Correct, and intended: I'd rather the Gradle build refuse to run on a wrong JDK than compile with whatever is on PATH. Note Maven isn't as loose as it looks either — --release 17 fails outright on a JDK older than 17, so the practical difference is only that Maven tolerates a newer JDK while Gradle pins exactly 17.
Co-Authored-By: Toby Drinkall <toby.drinkall@cognition.ai>
| @Bean | ||
| public CommandLineRunner run(RestTemplate restTemplate) throws Exception { | ||
| return args -> { | ||
| Quote quote = restTemplate.getForObject( | ||
| "http://gturnquist-quoters.cfapps.io/api/random", Quote.class); | ||
| log.info(quote.toString()); | ||
| }; | ||
| return args -> logRandomQuote(restTemplate); |
There was a problem hiding this comment.
📝 Info: Two CommandLineRunner beans still run in an unspecified order
Application both implements CommandLineRunner (JDBC demo) and exposes a second CommandLineRunner bean (run(RestTemplate)). Neither is annotated with @Order, so ordering is unspecified; this is unchanged by the PR but worth noting since the quote runner is now the only place that swallows network failures — the JDBC runner still aborts startup on any SQL error.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Pre-existing, unchanged, and left alone. The @Order gap doesn't matter here: the two runners share no state, one seeds an in-memory H2 table and the other logs a quote. The asymmetry you point out is deliberate — the quote fetch depends on a third-party host that no longer exists, so it must not be fatal, whereas a SQL failure against the embedded H2 the app just provisioned is a real defect and should still abort startup.
Summary
Java 8 → 17, Spring Boot 2.0.2 → 2.7.18, across both build systems. Two pre-existing breakages had to be fixed first, otherwise the upgrade looks like the regression:
<packaging>pom</packaging>, so no executable jar was ever produced → nowjar.spring-boot-starter-jdbc/h2even thoughApplicationimportsJdbcTemplate, so./gradlew bootJarcould not compile at all → both added.Also mandatory, not cosmetic: Maven wrapper 3.3.9 → 3.9.6 and Gradle wrapper 4.6 → 7.6.4 (neither old version runs on JDK 17). Gradle 7 removed the
compile/testCompileconfigurations and thebootJar { baseName/version }properties, henceimplementation/archiveBaseName.settings.gradleis new — without it Gradle 7 derives the project name from the checkout directory.spring-boot-starter-parentsuppliesmaven.compiler.source/targetfromjava.versionbut has noreleaseproperty, so--release 17needs an explicit<maven.compiler.release>17</maven.compiler.release>; the parent also pins compiler/surefire lower than what's wanted, so both are overridden via version properties.The app previously could not start at all, independent of Java version:
gturnquist-quoters.cfapps.iono longer resolves (Pivotal retired cfapps.io) and it was called frommain()and from aCommandLineRunner, i.e. insideSpringApplication.run. Now degraded instead of fatal:One other source change:
jdbcTemplate.query(sql, Object[], RowMapper)is deprecated in Spring 5.3 → varargs formquery(sql, rowMapper, "Josh"). The legacy H2 SQL (DROP TABLE customers IF EXISTS,SERIAL) is still accepted by H2 2.1.214, so it is unchanged.Repo hygiene, in the same commit but reviewable separately: untracked 14 stale Java 8
.classfiles undertarget/(and addedtarget/,build/,.gradle/to.gitignore), deleted the misnamed.gitignore.txtduplicate, movedapplication.propertiesfrom the repo root intosrc/main/resources/so it is actually on the classpath, and set the executable bit onmvnw/gradlew.temp.txtis left alone —TopicService.readFileWithStreamFunction()reads it from the working directory.Verified
On JDK 17.0.13:
mvn -B clean package→target/gs-spring-boot-0.1.0.jar./gradlew clean bootJar→build/libs/gs-spring-boot-0.1.0.jarGET /→{"id":1,"content":"Hello, World!"},GET /topic→ the topic list, H2 table creation + batch insert + query all run (Customer{id=3, firstName='Josh', ...}), and the dead quote host produces a single WARN rather than a startup failure.spring-boot-properties-migratoris still on the runtime classpath and reported no renamed/removed properties on startup.Not verified
There are no tests and no CI in this repo, so nothing here is test-backed — "verified" means compiles under
--release 17, packages, and starts, exercised by the manual requests above. The remaining Java 8 feature demos (NIO walk/find,java.time, the other/topic/*endpoints) were not individually exercised.Link to Devin session: https://app.devin.ai/sessions/8d8bcf0a2fa34059930d84303e9245f1
Requested by: @tobydrinkall
Devin Review